I compiled and tested this, and it works correctly. As I'm not familiar with java/scala, I cant provide a full review unfortunately. I did notice some warnings when starting cromwell, but as everything works, maybe that's not a problem ?

2021-03-13 12:17:25,630 WARN - Unrecognized configuration key(s) for AwsBatch: auth, numCreateDefinitionAttempts, default-runtime-attributes.awsBatchRetryAttempts, awsBatchRetryAttempts, filesystems.s3.duplication-strategy, numSubmitAttempts, default-runtime-attributes.scriptBucketName

Thanks by the way ! This was exactly what we were waiting for

I just added some documentation.

Seems like this is exactly what I need. This branch is not merged yet. Can i clone the repo, checkout this branch and build?

and can you confirm that the right way to set ulimits in the runtime block would e.g. be

runtime { ulimits = [ { "name": string, "softLimit": integer, "hardLimit": integer } ] }

@Irsan88 exactly. Checkout this branch and run the wdl with something like this:

"ulimits": [
      {
        "name": "nofile",
        "softLimit": "9000",
        "hardLimit": "9000"
      }
    ],

Make sure that the softLimit and hardLimit are string and not integers

I am trying to build this branch but got the error below. Both local (macOS) and cromwell-dev Docker image.

sbt assembly

[error] /code/wom/src/main/scala/wom/views/GraphPrint.scala:113:57: type mismatch;
[error]  found   : java.util.stream.Stream[String]
[error]  required: scala.collection.GenTraversableOnce[?]
[error]           |  ${digraph.links.toList.flatMap(_.dotString.lines).mkString(System.lineSeparator + "  ")}
[error]                                                         ^
[error] /code/wom/src/main/scala/wom/views/GraphPrint.scala:116:57: type mismatch;
[error]  found   : java.util.stream.Stream[String]
[error]  required: scala.collection.GenTraversableOnce[?]
[error]           |  ${digraph.nodes.toList.flatMap(_.dotString.lines).mkString(System.lineSeparator + "  ")}
[error]                                                         ^
[error] /code/wom/src/main/scala/wom/views/GraphPrint.scala:159:49: type mismatch;
[error]  found   : java.util.stream.Stream[String]
[error]  required: scala.collection.GenTraversableOnce[?]
[error]           |  ${nodes.toList.flatMap(_.dotString.lines).mkString(System.lineSeparator() + "  ")}
[error]                                                 ^
[error] /code/wom/src/main/scala/wom/views/GraphPrint.scala:168:48: type mismatch;
[error]  found   : java.util.stream.Stream[String]
[error]  required: scala.collection.GenTraversableOnce[?]
[error]          |  ${nodes.toList.flatMap(_.dotString.lines).mkString(System.lineSeparator() + "  ")}
[error]                                                ^
[error] four errors found

Any suggestion?

@wdesouza I am seeing this as well. This fork was created just before #6194 that upgraded Cromwell's Java version from 8 to 11. I think these compilation errors may represent some (hopefully minor) incompatibilities.

@mcovarr @wdesouza I will try to update this pr asap

I've changed _.dotString.lines to _.dotString.linesIterator and it worked. Thanks @henriqueribeiro and @mcovarr

Hi @henriqueribeiro ,

Do you think I can compile this branch with functionality of the latest cromwell release (77) ? I'm now using the branch based on cromwell 58, but more recent versions have retry strategy that's interesting as well:

• you retry logic handles spot kills
• cromwell retry handles the fetch_and_run is a directory problem.

=> both would be great, but since it doesn't get approved, I hope to make a new custom build. However, it says here there are conflicts...

Greetings,
geert

Hey @geertvandeweyer. I'm preparing a new release with more functionalities based on cromwell 78. It should be ready in the next few days.

Are you in the cromwell slack?

@henriqueribeiro what's the status of this. There's a new PR that seems to do something similar.. :)
#7675
What is left for this to be awesome (had a quick glance, hope I did not miss it) is to define retryStrategies as well, so that you can decide what to do depending on whether the spot instance was reclaimed (infra error) or the script failed (user error).

Hi,

We maintain an aws specific fork that has these functionalities . Spot kills are handled separately from program errors

Check it out here:

https://github.com/henriqueribeiro/cromwell

Or my latest dev release forked from that one :

https://github.com/geertvandeweyer/cromwell

Best,
Geert

Any chance this can be upstreamed? An issue we can ask our customers to +1 to motivate that?

I'm not entirely sure of how/why the AWS fork came about, but fixing all of these conflicts and upstreaming don't seem like things we'd have capacity for at present.

I have some back story on how the fork came about. For a while I was working with Broad on AWS Batch integration with Cromwell. At some point Broad stopped reviewing PRs for this, probably due to the focus on Terra. For this reason the fork was made. When we supported Cromwell as an option with AGC we maintained this fork however with AGC now deprecated in favor of HealthOmics we no longer maintain the fork. Others wanting to continue using Cromwell on AWS Batch built on that fork. The best option for that case is the https://github.com/geertvandeweyer/cromwell fork.

The AWS HealthOmics service WDL engine is based on miniwdl so we no longer need to make any official merge of this older fork. It is also probably seriously out of date with the HEAD of main. Gert's fork may be more easily merged?

Thanks for the update, that is helpful to know.

It may be possible for us to just merge PRs constrained to supportedBackends/aws/ without rigamarole, since we don't use it in Terra.

Hi,

Thanks for that feedback. I would absolutely be in favor of merging the functionality back into the original repo. If we get support if all changes are restricted to the AWS specific files, we can try to achieve that. Right now there are some changes in the general code base as well.